Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

asynchronously resolve a name to multiple address objects #548

Merged
merged 42 commits into from
Nov 26, 2016

Conversation

glyph
Copy link
Member

@glyph glyph commented Sep 22, 2016

@codecov-io
Copy link

codecov-io commented Oct 18, 2016

Current coverage is 91.15% (diff: 100%)

Merging #548 into trunk will increase coverage by 0.02%

@@              trunk       #548   diff @@
==========================================
  Files           834        836     +2   
  Lines        145653     145992   +339   
  Methods           0          0          
  Messages          0          0          
  Branches      12913      12930    +17   
==========================================
+ Hits         132735     133080   +345   
+ Misses        10664      10660     -4   
+ Partials       2254       2252     -2   

Powered by Codecov. Last update d12bb01...6625d4b

@glyph
Copy link
Member Author

glyph commented Nov 18, 2016

I am bumping into this issue twisted/twistedchecker#44 while writing the docstrings for this.

Copy link
Member

@hawkowl hawkowl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pls look at these things and merge when you are happy, seems good to me


from __future__ import division, absolute_import

__metaclass__ = type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid accidentally declaring an old-style class.

@return: The resolution in progress.
@rtype: L{IResolutionReceiver}
"""

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems like too many newlines


from __future__ import division, absolute_import

__metaclass__ = type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again, what's this for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. It avoids accidentally declaring an old-style class in py2.

@hawkowl
Copy link
Member

hawkowl commented Nov 26, 2016 via email

@glyph
Copy link
Member Author

glyph commented Nov 26, 2016

Why not add it to the list in t.test.test_nooldstyle or whatever?

It's kind of a separate issue. Although that would also be a good thing.

This list in test_nooldstyle has no docstring, though. It should have an @ivar so I know exactly what to do with it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants